Patch-stack rework: content-based trigger, richer reporting, no quiltimport - #43

Six commits.

1. Add PGBuild::PatchSeries

The code that reads a quilt-style patch repository — resolving a series entry
to the patch it actually names, parsing series, materializing the result —
existed as two private copies, in PGBuild/Modules/PatchStack.pm and
check_patch_stack.pl. They had already drifted: the same symlink fix had to
land twice (9039004 and cb961c8), and the two series parsers disagreed about
an indented entry, which the buildfarm silently dropped and the checker
honoured. That last one is fixed here.

2. Trigger on resolved series content

A branch whose series names a patch in another branch's subdirectory, as
../master/foo.patch, was not rebuilt when that patch changed. The trigger was
the git tree SHA of the branch's own subdirectory, and that does not move in
this case — the series blob still holds the same text. The series was applied
every run regardless, so a stack that stopped applying was still reported; what
did not happen was rebuilding and retesting the branch against the changed
patch.

It is now a digest over the resolved blob SHA of every patch the series names.
Patches a branch does not name contribute nothing, so a push touching one
branch's stack still does not rebuild the others.

patch_stack.log gains a format marker, the patches-repo commit that was used,
and a blob SHA per patch. check_patch_stack.pl reads the stack through the
shared module and gains --manifest, which checks that every series entry
resolves to a file that is actually present — it needs no buildroot, so it is
cheap to run before pushing a stack change.

3. Replace git quiltimport

Three problems with it, each reproduced rather than assumed:

• It skips a series entry whose patch file is absent and exits zero, so
  a branch could build and report a green result with a patch missing from its
  stack.
• It is not the git am wrapper its name suggests. It uses
  $GIT_DIR/rebase-apply as its own scratch directory and does not remove it
  when it fails, leaving a repository git reports as mid-rebase which can be
  neither continued nor aborted.
• The commit it creates per patch reached nothing: log_id() writes a headref
  captured at checkout, never a fresh rev-parse.

The series is now walked directly, applying each patch with git apply and
stopping at the first entry that is missing or fails. No commits are created
and no rebase state is written, so the stranded rebase cannot recur.
cleanup restores the tree with reset --hard and clean -fd.

check_patch_stack.pl --sequential calls the same code, so it predicts a real
run rather than approximating one. It consequently stops falling back from
-p1 to -p0; the default mode keeps that, since it checks each patch against
the pristine base in isolation.

4. Stop reading a strip level out of a series line's comment

parse_series() scanned every token on a series line for a -pN, so a level
written after the # that starts a comment was taken as real. The security
stacks keep a redmine id there, and a -p2 in that text became the strip level
the patch was applied at — a level neither quilt nor git quiltimport would
use, since both treat everything past the marker as comment.

Strip levels do not contribute to the series digest, so this moves no branch's
identity and triggers no rebuilds. Checked against the live security stacks: no
series line has that shape today, so the change is inert there and only stops
the divergence appearing later.

5. Apply the series with the patches repo's own driver

Per Noah Misch's point on the thread: the tool that applies the stack should
live in the patches repo and be called by the client, not be part of the
client, so that what an animal tests is what a release wrap produces rather
than something resembling it. A patch git apply takes and git am does not —
one with no usable From: or Subject: — passes on every animal and fails on
wrap day.

The client now looks for import-series.pl at the top of the patches repo and
hands it the resolved series when it is there. It gets the materialized copy,
so a shared ../master/foo.patch entry or a symlink is an ordinary file by the
time the script sees one — the same thing it sees on wrap day against a real
checkout. Choosing branches, fetching, resetting to upstream and deciding
whether to run at all stay on the client side.

The driver applies with git am, so HEAD moves and a commit is created per
patch. The client records the commit the tree started on, resets to that in
cleanup, and first clears any git am state a failed entry left behind,
which a reset does not remove. It supplies a committer identity when git cannot
find one of its own, since an animal's git is frequently unconfigured and git's
guess from the host name fails outright on a host with no domain; a configured
identity is left alone, and a patch's own From: wins either way.

The name comes from a new driver setting, defaulting to import-series.pl.
A name set in the config has to be there — naming one says the repo has an
applier of its own, and falling back from a name that turned out to be wrong
would apply the stack with something other than what the config asked for and
still report green. Only the default name may be absent, and then the built-in
loop runs. That is not a transition measure: PatchStack is a general module,
and a quilt-style repo that no release wrap consumes has no reason to carry a
driver. patch_stack.log records which of the two ran, as
patch_stack_applier; PatchStackLog.pm skips keys it does not know, so an
existing server ignores it.

6. Check the driver's reading of series against ours

Two programs parse that file — the client because the rebuild digest and the
list of blobs to materialize come out of it, the driver because its reading is
what gets applied — and nothing noticed when they differed. They did: commit 4
above is exactly that case.

The driver is run with --list before anything is applied. It prints its own
reading and exits without touching the tree: one line per entry, in series
order, name and strip level separated by a tab, - where the line gave no
level. A disagreement is reported with both readings and nothing applied. A
driver that does not understand --list is reported the same way rather than
skipping the check — a check that quietly passes when the other side is
unfamiliar is the failure it exists to prevent.

Verified by reverting commit 4 and re-running: the comment-strip case turns from
"applied at -p2 and reported green" into a reported disagreement.

Deploying this

Safe to run before the driver exists. With no import-series.pl in the patches
repo and no driver in the config, the lookup fails and the built-in git apply loop runs, exactly as it did before commits 5 and 6. The day the script
lands in the quilt repo, animals already on this client pick it up on their next
run with no config change, while animals on an older client carry on with git apply.

The driver itself is not in this PR — it belongs in the patches repo. Drafts
of it, and of the wrap-day scripts that call it, are written and tested but not
yet proposed.

One behaviour change worth arguing about

How much drift in surrounding context a patch may have now depends on where it
came from. A patch stored in a branch's own subdirectory is applied with full
context: it was written for that branch, so if it no longer applies exactly,
upstream has moved beneath the stack and that is worth reporting rather than
absorbing. An entry reaching into another branch's subdirectory keeps the
looser matching, since a patch written against one branch and applied to
another has an obvious reason to find its surroundings a little different.

Ordering

PGBuildFarm/server-code#20 has merged, so the reporting half has a server that
renders it and this is the remaining piece.

Not yet exercised on a real animal.